added unit tests for rolldown @drwpow - thanks for the suggestion!

thanks for offering to take it up @askoufis! but i resolved the conflicts - didn't look like there were too many of them.

i also duplicated the same extract test under rolldown describe section

and thanks again for taking the time to review this

It looks like rolldown isn't stripping side-effect imports for some reason.

Here's the equivalent section of the rollup snapshot in master:

Any thoughts @drwpow?

Hm yeah we go in and remove those after-the-fact if someone is extracting .css. That happens here, in generateCssBundle():

  for (const id of getModuleIds()) {
    if (cssFileFilter.test(id)) {
      cssFiles[id] = buildImportChain(id, { getModuleInfo, warn });
    }
  }

Ultimately, if they get added to that cssFiles mapping, then we’ll strip the side effect imports later.

I haven’t debugged locally, but what is likely happening is Rolldown doesn’t have a complete graph of this.getModuleIds() at this point in time, so we don’t know that these are side-effects we generated (we obviously don’t touch user code as that would be Bad; we only will remove imports we know Vanilla Extract generated).

If, say, we debugged what the current status is of this.getModuleIds() at this point in time:

+   console.log(getModuleIds());
    for (const id of getModuleIds()) {

I’d expect to see that for whatever reason, Rollup and Rolldown are returning different things here.

If so, maybe it’s because Rolldown hasn’t yet built this graph, or it’s possibly partial, or something. All that tracks, because Rolldown has intentionally departed from some of Rollup’s API which led to this PR in the first place. I may not be able to take a look this week, but it could be as simple as calling generateCssBundle() at some other point in time, maybe.

I think I remember the file output is not exactly the same in rolldown as it is in rollup

So the file we're looking for might just have been renamed or something internally by rolldown

I guess the strategy to get extractedCssIds has to be looked at

Oh! Yeah if that’s the case then we can just expand our RegEx to look for both Rollup and Rolldown versions. That still should be pretty safe/I wouldn’t imagine any user code getting accidentally stripped from it (we have multiple checks to ensure we’re not just deleting user code, that’s only one layer).

ok so i see what's going on - we're stripping side effects manually - and previously in rollup it was kinda collapsing the modules where needed whereas in rolldown it's not doing that anymore

so rollup produces something like this:

- app.js
- reset.css

and app.js imports reset.css directly removing the interim reset.css.ts file since i guess rollup sees that the interm js file is not needed as it's just side-effects and doesn't export any actual js modules

whereas in rolldown we have:

- app.js
- reset.js
  - reset.css 

app.js is importing reset.js (which was reset.css.ts) and that in turn is importing reset.css. the side effects is now getting stripped from reset.js as you can see here:

https://github.com/radnan/vanilla-extract/blob/d385965a8057dff0ba8ecc9b8fe1de8e780db267/packages/rollup-plugin/test/__snapshots__/rollup-plugin.test.ts.snap#L1578-L1581

but we're left with an empty reset.js file that does nothing. i guess we'd have to do a second pass to manually remove these empty js modules - or just live with it

Oh good find. Yeah I wonder given Rolldown’s current state and active iteration, if we just leave it as-is because I would strongly imagine others run into this, and this gets fixed on Rolldown’s side? Maybe?